Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run mlx-ui as non-root user #339

Merged
merged 1 commit into from
Jun 15, 2022

Conversation

ckadner
Copy link
Member

@ckadner ckadner commented Jun 14, 2022

Run the MLX UI container as node user not as root.

Also update to Node 16

Closes: #337

Related: IBM/manifests#43

FYI @Tomcli @yhwang -- this worked on OC on Fyre after undoing the mlx-ui manifest change for adding the emptyDir build-volume. I created a PR #43 on the manifest repo for it.

@ckadner ckadner requested a review from yhwang June 14, 2022 22:04
@mlx-bot
Copy link
Collaborator

mlx-bot commented Jun 14, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ckadner

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mlx-bot mlx-bot requested a review from Tomcli June 14, 2022 22:04
@ckadner ckadner added UI User Interface and removed approved labels Jun 14, 2022
@ckadner
Copy link
Member Author

ckadner commented Jun 14, 2022

@jbusche

@ckadner
Copy link
Member Author

ckadner commented Jun 14, 2022

FYI @Tomcli @yhwang -- this worked on OC on Fyre after undoing the mlx-ui manifest change for adding the build-volume. I created a PR on the manifest repo for it:

IBM/manifests#43

ckadner added a commit to ckadner/manifests that referenced this pull request Jun 14, 2022
After changes to the mlx-ui Docker image, we no longer
require the externally mounted build volume

Related: machine-learning-exchange/mlx#339

Signed-off-by: Christian Kadner <[email protected]>
@jbusche
Copy link
Contributor

jbusche commented Jun 14, 2022

Tested with @ckadner on a OC 4.10 cluster, looking good:

  1. The user remains non-root even after multiple pod restarts:
oc rsh mlx-ui-5c48666cf7-59hc8
$ whoami
node
  1. The pod starts correctly:
oc logs -f mlx-ui-5c48666cf7-2cw8m

> [email protected] build
> react-scripts build

Creating an optimized production build...
Compiled successfully.

File sizes after gzip:

  343.26 KB  build/static/js/2.6e8d8be7.chunk.js
  62.84 KB   build/static/js/main.2854922d.chunk.js
  3.84 KB    build/static/css/main.265e3da4.chunk.css
  2.72 KB    build/static/css/2.21151bd0.chunk.css
  780 B      build/static/js/runtime-main.b8b6e910.js

The project was built assuming it is hosted at ./.
You can control this with the homepage field in your package.json.

The build folder is ready to be deployed.

Find out more about deployment here:

  bit.ly/CRA-deploy

npm notice 
npm notice New minor version of npm available! 8.11.0 -> 8.12.1
npm notice Changelog: <https://github.com/npm/cli/releases/tag/v8.12.1>
npm notice Run `npm install -g [email protected]` to update!
npm notice 
[HPM] Proxy created: /  ->  http://mlx-api
Server listening at http://localhost:3000

@yhwang
Copy link
Member

yhwang commented Jun 15, 2022

question: I thought you want to use package-lock.json to lock all package versions since you update two package-lock.json files. However, when building the docker image, you remove the lock files before calling npm i. I guess it defeats the purpose of updating those package-lock.json files...

@ckadner
Copy link
Member Author

ckadner commented Jun 15, 2022

question: I thought you want to use package-lock.json to lock all package versions since you update two package-lock.json files. However, when building the docker image, you remove the lock files before calling npm i. I guess it defeats the purpose of updating those package-lock.json files...

I can remove that. I put that in because the npm install can fail depending on the content of package-lock.json and the version of npm used by the user who last ran the npm install locally (overriding the package-lock.json last checked into VCS) -- in some cases bringing up a npm login prompt. There also have been changes over time in the way the package-lock.json file is being used by npm ... https://stackoverflow.com/questions/44297803/what-is-the-role-of-the-package-lock-json

@yhwang
Copy link
Member

yhwang commented Jun 15, 2022

when package-lock.json exits, npm i uses the versions in that files. it shouldn't change the lock file unless someone changes package.json and want to rebuild the lock file. my thinking is when using lock files, we should only do npm i

@ckadner
Copy link
Member Author

ckadner commented Jun 15, 2022

when package-lock.json exits, npm i uses the versions in that files. it shouldn't change the lock file unless someone changes package.json and want to rebuild the lock file. my thinking is when using lock files, we should only do npm i

Are you referring to npm install (npm i) vs npm ci https://stackoverflow.com/questions/52499617/what-is-the-difference-between-npm-install-and-npm-ci ?

@yhwang
Copy link
Member

yhwang commented Jun 15, 2022

/lgtm

@mlx-bot mlx-bot added the lgtm label Jun 15, 2022
@mlx-bot mlx-bot merged commit d78055e into machine-learning-exchange:main Jun 15, 2022
yhwang pushed a commit to IBM/manifests that referenced this pull request Jun 15, 2022
After changes to the mlx-ui Docker image, we no longer
require the externally mounted build volume

Related: machine-learning-exchange/mlx#339

Signed-off-by: Christian Kadner <[email protected]>
mlx-bot-app bot pushed a commit that referenced this pull request Oct 28, 2022
* Add dev doc links to CONTRIBUTING.md (#312)

Resolves a task from #304

Signed-off-by: Rafael Vasquez <[email protected]>

* Fixes the references to latest kfctl release (#311)

Resolves a task from #304

Signed-off-by: Rafael Vasquez <[email protected]>

* Do not verify links in project dependencies (#320)

Resolves #319

Signed-off-by: Christian Kadner <[email protected]>

* Add navigation and description table of docs (#314)

* Create readme for docs
* Add description table to readme
* Add links to table
* update relative links
* Change documentation to document

Signed-off-by: Rafael Vasquez <[email protected]>

* Add script to update the docs table  (#317)


Signed-off-by: Rafael Vasquez <[email protected]>

* Modified the way sed version is set (#315)

Instead of checking the operating system or shell emulator,
test which version of `sed` is actually installed in the local
environment.

Resolves #301

Signed-off-by: Krishna Kumar <[email protected]>

* Bump waitress from 2.0.0 to 2.1.1 in /api/server (#321)

Bumps [waitress](https://github.com/Pylons/waitress) from 2.0.0 to 2.1.1.
- [Release notes](https://github.com/Pylons/waitress/releases)
- [Changelog](https://github.com/Pylons/waitress/blob/master/CHANGES.txt)
- [Commits](Pylons/waitress@v2.0.0...v2.1.1)

---
updated-dependencies:
- dependency-name: waitress
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump jupyter-server from 1.13.4 to 1.15.4 in /api/server (#324)

Bumps [jupyter-server](https://github.com/jupyter/jupyter_server) from 1.13.4 to 1.15.4.
- [Release notes](https://github.com/jupyter/jupyter_server/releases)
- [Changelog](https://github.com/jupyter-server/jupyter_server/blob/main/CHANGELOG.md)
- [Commits](jupyter-server/jupyter_server@v1.13.4...v1.15.4)

---
updated-dependencies:
- dependency-name: jupyter-server
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump minimist from 1.2.5 to 1.2.6 in /dashboard/origin-mlx (#326)

Bumps [minimist](https://github.com/substack/minimist) from 1.2.5 to 1.2.6.
- [Release notes](https://github.com/substack/minimist/releases)
- [Commits](https://github.com/substack/minimist/compare/1.2.5...1.2.6)

---
updated-dependencies:
- dependency-name: minimist
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump notebook from 6.4.8 to 6.4.10 in /api/server (#327)

Bumps [notebook](http://jupyter.org) from 6.4.8 to 6.4.10.

---
updated-dependencies:
- dependency-name: notebook
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Update API developer docs (#325)

* Add codegen workflow diagram
* Describe codegen workflow
* Describe API server package modules

Closes #325

Signed-off-by: Pavan Pss <[email protected]>

Co-authored-by: Christian Kadner <[email protected]>

* Update UI developer docs (#323)

Closes #323

Signed-off-by: RRM123 <[email protected]>

* Correct description for make update_doc_table (#329)

Signed-off-by: ezinneanne <[email protected]>

* Update Kubernetes high version in deployment docs (#318) (#332)

* Add Troubleshooting section
* Limit K8s version for KIND cluster using `--image` flag

Resolves #318

Signed-off-by: Kiran-Patel <[email protected]>
Signed-off-by: Kiran Patel [email protected]
Signed-off-by: Christian Kadner <[email protected]>

Co-authored-by: Kiran-Patel <[email protected]>
Co-authored-by: Christian Kadner <[email protected]>

* fix errors in mlx-ui startup (#338)

* Run mlx-ui as non-root user (#339)

Closes #337

Signed-off-by: Christian Kadner <[email protected]>

* Update MLX setup instructions for KF 1.5 (#346)

* Update MLX setup instructions for KF 1.5

Signed-off-by: Christian Kadner <[email protected]>

* fix selected assets show up in different menus (#342)

Signed-off-by: Jiaxuan-Yang <[email protected]>

* Consolidate readme (#356)

Signed-off-by: Rafael Vasquez <[email protected]>

Signed-off-by: Rafael Vasquez <[email protected]>
Co-authored-by: Rafael Vasquez <[email protected]>

* Document MLX Models Workshop (#352)

Signed-off-by: Christian Kadner <[email protected]>

* Add GitHub action to verify doc links (#357)

Signed-off-by: Rafael Vasquez <[email protected]>

Signed-off-by: Rafael Vasquez <[email protected]>
Signed-off-by: Christian Kadner <[email protected]>
Signed-off-by: Krishna Kumar <[email protected]>
Signed-off-by: RRM123 <[email protected]>
Signed-off-by: ezinneanne <[email protected]>
Signed-off-by: Jiaxuan-Yang <[email protected]>
Signed-off-by: Rafael Vasquez <[email protected]>
Co-authored-by: Rafael Vasquez <[email protected]>
Co-authored-by: Krishna Kumar Ramachandran <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Pavan Pss <[email protected]>
Co-authored-by: RRM123 <[email protected]>
Co-authored-by: Ezinne Anne Emilia <[email protected]>
Co-authored-by: Kiran Patel <[email protected]>
Co-authored-by: Kiran-Patel <[email protected]>
Co-authored-by: jbusche <[email protected]>
Co-authored-by: Joanna <[email protected]>
Co-authored-by: Rafael Vasquez <[email protected]>
yhwang pushed a commit to yhwang/manifests that referenced this pull request Aug 10, 2023
After changes to the mlx-ui Docker image, we no longer
require the externally mounted build volume

Related: machine-learning-exchange/mlx#339

Signed-off-by: Christian Kadner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mlx ui pod: "EACCES: permission denied, mkdir '/workspace/build'"
4 participants